fix: add opf-load-once capability on js and css resources #19825
Conversation
| protected readonly CORS_DEFAULT_VALUE = 'anonymous'; | ||
| protected readonly OPF_RESOURCE_LOAD_ONCE_ATTR_KEY = 'opf-load-once'; |
There was a problem hiding this comment.
Could we use for consistency: OPF_RESOURCE_LOAD_ONCE_ATTRIBUTE_KEY?
There was a problem hiding this comment.
Also would be good in my opinion to prefix all custom html attributes with "data-".
https://www.w3schools.com/tags/att_data-.asp
Can we change attribute key to: data-opf-load-once?
There was a problem hiding this comment.
No need to add 'data-' prefix:
'opf-load-once' is never added as attribute key, it is just a variable that decides if we add 'data-opf-resource' attribute or not.
To sum-up, we receive it in attributes property from server but using it as a flag in this service.
|
|
||
| protected embedStyles(embedOptions: { | ||
| attributes?: OpfKeyValueMap[]; | ||
| attributes?: { [key: string]: string }; |
There was a problem hiding this comment.
Is it new type different than OpfKeyValueMap[]?
There was a problem hiding this comment.
No, it was set as 'any'.
OpfKeyValueMap is {key:string,value:string}, we receive the list in this format from resource object (resource.attributes).
We perform a conversion to [key: string]: string to make handling easier and have same type as attributes in ScriptLoader service
| @@ -82,31 +103,21 @@ export class OpfResourceLoaderService { | |||
| */ | |||
| protected loadScript(resource: OpfDynamicScriptResource): Promise<void> { | |||
| return new Promise((resolve, reject) => { | |||
| const attributes: any = { | |||
| const attributes: { [key: string]: string } = { | |||
There was a problem hiding this comment.
Can we create type for it in a model if we cannot use: OpfKeyValueMap?
There was a problem hiding this comment.
Mentioned above, we need both:
OpfKeyValueMap is the original format (resource.attributes received from server)
{ [key: string]: string } is format needed for ScriptLoaderService
Thus a conversion is needed.
integration-libs/opf/base/root/services/opf-resource-loader.service.ts
Outdated
Show resolved
Hide resolved
spartacus
|
||||||||||||||||||||||||||||
| Project |
spartacus
|
| Branch Review |
feature/CXSPA-8883_v2
|
| Run status |
|
| Run duration | 03m 50s |
| Commit |
|
| Committer | Florent Letendre |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
3
|
Pending
|
2
|
Skipped
|
0
|
Passing
|
125
|
| View all changes introduced in this branch ↗︎ | |
No description provided.